[cDAC] Reshape DacDbi API GetHeapSegments to EnumerateHeapSegments and implement in cDAC#128054
Open
Copilot wants to merge 4 commits into
Open
[cDAC] Reshape DacDbi API GetHeapSegments to EnumerateHeapSegments and implement in cDAC#128054Copilot wants to merge 4 commits into
Copilot wants to merge 4 commits into
Conversation
…; implement in cDAC Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
rcj1
changed the title
rcj1
changed the title
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the DAC/DBI heap segment API pattern from “return an allocated array” (GetHeapSegments) to “enumerate via callback” (EnumerateHeapSegments) and wires up a corresponding managed cDAC implementation, contract support, and tests/mocks.
Changes:
- Replaces
IDacDbiInterface::GetHeapSegmentswithEnumerateHeapSegments(fpCallback, pUserData)across IDL/header, native DAC implementation, and RS consumption. - Adds
IGC.EnumerateHeapSegments(GCHeapData)plus supporting types (GCSegmentClassification,GCHeapSegmentInfo) and implements the walk inGC_1. - Extends the cDAC test harness with a mock
HeapSegmentdescriptor and adds unit tests covering workstation/server + regions/segments behaviors.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs | Adds a mock HeapSegment layout and builder helpers to construct segment lists in tests. |
| src/native/managed/cdac/tests/GCTests.cs | Adds unit tests validating segment enumeration behavior for wks/svr and regions/segments modes. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs | Updates the COM surface to EnumerateHeapSegments and introduces CorDebugGenerationTypes for segment classification. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs | Implements EnumerateHeapSegments in the managed cDAC DBI shim and adds DEBUG parity checking vs legacy DAC. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/GC_1.cs | Implements raw per-heap segment list walking and classification (regions vs segments, readonly/non-GC, ephemeral marker). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IGC.cs | Adds new public contract types and the EnumerateHeapSegments API. |
| src/coreclr/inc/dacdbi.idl | Replaces GetHeapSegments with callback-based EnumerateHeapSegments and defines the callback typedef. |
| src/coreclr/debug/inc/dacdbiinterface.h | Updates the native interface declaration and documents callback semantics. |
| src/coreclr/debug/di/process.cpp | Converts RS heap-region enumeration to use the callback API and accumulates segments locally. |
| src/coreclr/debug/daccess/dacdbiimpl.h | Updates the DAC implementation declaration to EnumerateHeapSegments. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Implements callback-based segment enumeration (no pre-count/alloc). |
| docs/design/datacontracts/GC.md | Documents the new contract API and segment classification/ephemeral-marker behavior. |
Copilot's findings
Comments suppressed due to low confidence (1)
docs/design/datacontracts/GC.md:318
- The new HEAP_SEGMENT_FLAGS_READONLY constant is documented as type 'ulong', but HeapSegment.Flags is a pointer-sized value (TargetNUInt) and the implementation uses a uint constant (1) for masking. Please align the documented type with the actual contract/implementation (e.g., document it as a bit value usable with pointer-sized Flags) to avoid confusion for contract consumers.
| `HEAP_SEGMENT_FLAGS_READONLY` | ulong | `HeapSegment.Flags` bit identifying a readonly (e.g. frozen, non-GC) segment. | `1` |
```csharp
GCHeapType IGC.GetGCIdentifiers()
{
- Files reviewed: 12/12 changed files
- Comments generated: 4
Open
3 tasks
noahfalk
approved these changes
May 14, 2026
Member
noahfalk
left a comment
noahfalk
left a comment
There was a problem hiding this comment.
LGTM 👍 A few small comments inline.
| { | ||
| // Bounded traversal of the singly-linked HeapSegment list, guarding against cycles or | ||
| // corrupt links via a fixed iteration cap (MaxSegmentListIterations = 2048). | ||
| int iterationMax = MaxSegmentListIterations; |
Member
There was a problem hiding this comment.
A large enough GC heap in regions mode can probably exceed this limit. Try a test app that creates 10+ GB of small objects to see how it behaves.
Contributor
There was a problem hiding this comment.
Good catch, with 10GB I get 2823 segments. In the native DAC we cap the number of regions at 8192. Maybe both should be just a little more liberal, say 16384?
Member
There was a problem hiding this comment.
The biggest app heaps I hear of today are in the 50-100GB range and we probably want to add headroom to grow even bigger. A limit at 50K segments would put us ~200GB.
|
|
||
| [PreserveSig] | ||
| int GetHeapSegments(nint pSegments); | ||
| int EnumerateHeapSegments(delegate* unmanaged<ulong, ulong, int, uint, nint, void> fpCallback, nint pUserData); |
Member
There was a problem hiding this comment.
Nit: since the C# function pointer syntax doesn't allow naming the parameters it might help clarity to include a comment describing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Existing native GetHeapSegments allocates - to avoid this, I reworked it into EnumerateHeapSegments which calls a callback on each segment to add it to a DacDbiArray structure (potentially allocating). There are a bunch of APIs like this.